Repository navigation
Conversation
|
The GCP retry passed on the same commit: 2,229 tests passed, 237 were skipped, and none failed. Both new fused-harness regression cases passed, as did the streaming-insert KMS test that previously received a BigQuery HTTP 500. All current-head checks are now complete: 18 successful and one neutral. |
b93bead to
02e8496
Compare
bvolpato
changed the title
|
Assigning reviewers: R: @kennknowles for label java. Note: If you would like to opt out of this review, comment Available commands:
The PR bot will only process comments in the main thread (not review comments). |
|
R: @ahmedabu98 |
|
Stopping reviewer notifications for this pull request: review requested by someone other than the bot, ceding control. If you'd like to restart, comment |
b607e9d to
f5dfbd4
Compare
bvolpato
changed the title
ahmedabu98
left a comment
ahmedabu98
left a comment
There was a problem hiding this comment.
This is a good fix for the normal case.
Please look into the case when withErrorHandler is set. When the writer flushes mid-bundle, any downstream error (from success-row propagation) will bubble up past this try-catch and into the try-catch in ConvertMessagesDoFn. The bundle succeeds and whatever row ConvertMessagesDoFn was working on upstream incorrectly goes to the DLQ and never reaches BQ.
I think we should do the same fix in ConvertMessagesDoFn and move .output() outside of the try-catch
|
waiting on author |
ahmedabu98
left a comment
ahmedabu98
left a comment
There was a problem hiding this comment.
LGTM, thanks for the fix Bruno!
3048fd2 to
9bbb5df
Compare
With success-row propagation enabled, the unsharded Storage Write API path can catch exceptions from fused downstream transforms as parsing or conversion errors. A bundle can report success even though a downstream consumer failed. With withErrorHandler, an unrelated input row can also go to the dead-letter output. Move successful output outside the parsing and conversion handlers so that downstream failures fail the bundle. Parsing and conversion errors keep their existing handling.
9f517da to
65f0674
Compare
With success-row propagation enabled, the unsharded BigQuery Storage Write API path can catch exceptions from fused downstream transforms as parsing or conversion errors. A bundle can report success even though a downstream consumer failed. With
withErrorHandler, an unrelated input row can also be sent to the dead-letter output.This change moves successful output outside the parsing and conversion handlers so downstream failures fail the bundle. Actual parsing and conversion errors keep their existing handling. With
STORAGE_API_AT_LEAST_ONCE, retrying a failed bundle can duplicate rows that BigQuery already accepted.The SDK harness regression covers mid-bundle and finish-bundle flushes, with and without an error handler. The targeted GCP tests and Spotless passed locally.
Run the GCP module tests with:
addresses #123orfixes #456).CHANGES.mdwith noteworthy changes.See CONTRIBUTING.md for contribution guidelines.